Add LRSOMATICREPORT as the final pipeline step - #176
Draft
ljwharbers wants to merge 9 commits into
Draft
Conversation
Wraps the lrsomatic_report R/Quarto tool (added as a git submodule) in a new local module that renders a self-contained per-sample HTML report from the pipeline's key outputs (VEP-annotated somatic SNVs, Severus SVs, ASCAT copy number, QC). Runs last, gated by --skip_report, so future dependents (e.g. a real Wakhan integration) can hook in without restructuring. Every report input is optional and joined by plain sample-id keys (not full meta maps) so missing/skipped upstream steps degrade gracefully instead of breaking the join. The module's environment.yml was verified against the actual R code (not just the tool's README) and trimmed to what's really used; a Wave container was built and validated with a real end-to-end render. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This PR is against the
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new final pipeline step (LRSOMATICREPORT) to render a per-sample, self-contained HTML report from the pipeline’s main outputs, controlled via --skip_report and report-related parameters.
Changes:
- Integrates
LRSOMATICREPORTas the final workflow step, joining optional upstream outputs to degrade gracefully when steps are skipped. - Introduces a new local module (
modules/local/lrsomaticreport) with conda/container support and nf-test coverage. - Adds new report parameters to
nextflow.config,nextflow_schema.json, and updates documentation and module configuration/publishing.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/lrsomatic.nf | Wires LRSOMATICREPORT into the main workflow and assembles/join report inputs. |
| nextflow.config | Adds skip_report, report_src, and report_gene_panel defaults. |
| nextflow_schema.json | Exposes report parameters in the schema (report_options) and adds skip_report. |
| conf/modules.config | Adds publishing configuration and parameter-to-args wiring for LRSOMATICREPORT. |
| modules/local/lrsomaticreport/main.nf | Implements the report-rendering process (staging inputs, running render_report.R). |
| modules/local/lrsomaticreport/environment.yml | Defines the conda environment used for the module. |
| modules/local/lrsomaticreport/meta.yml | Adds nf-core style metadata for the new module. |
| modules/local/lrsomaticreport/tests/main.nf.test | Adds nf-test coverage (stub + real render) for the module. |
| modules/local/lrsomaticreport/tests/main.nf.test.snap | Snapshot output for the stub test. |
| docs/usage.md | Documents --skip_report, --report_src, and --report_gene_panel. |
| docs/output.md | Documents the new per-sample report/ output directory and HTML artifact. |
| .gitmodules | Adds the assets/lrsomatic_report submodule definition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CI's docker/singularity 25.04.0 jobs regressed vs. dev after adding the
report step. Chasing the failures end-to-end (real render, not just
config) surfaced five distinct bugs:
1. workflows/lrsomatic.nf: normal-sample QC was keyed by the boolean
meta.paired_data instead of meta.id, so the join produced a
malformed remainder tuple and crashed the pipeline for any matched
tumor/normal pair reaching the report step. Also fixed misleading
comments (paired_data is not a sample id).
2. modules/local/lrsomaticreport/main.nf: qc_tumor_files/qc_normal_files
were staged flat. mosdepth/samtools default to a meta.id-only
prefix, so a matched pair's tumor and normal QC files share a name
and collided in the task work dir. Fixed via stageAs subdirectories
(qc_tumor/*, qc_normal/*) with basename-based destination linking.
3. modules/local/lrsomaticreport/environment.yml: missing r-r.utils,
needed by data.table::fread() to read a gzipped VCF directly --
only surfaced once a real Severus VCF reached the render step.
Required rebuilding the Wave container (new frozen tag
4506737a6b63b769); the container directive now follows this
codebase's existing dual-engine pattern (singularity blob URL +
docker tag, e.g. modules/local/bcftools/view/main.nf) since the
frozen singularity artifact is SIF/ORAS-native, not a portable OCI
image.
4. assets/lrsomatic_report submodule (re-pinned to fdf2a0a):
parse_severus_vcf's fread() errored instead of returning zero rows
when a sample's Severus VCF has no variant records at all (skip
landing exactly on the last line). Fixed upstream with tryCatch.
5. modules/local/lrsomaticreport/main.nf: report_src was staged via a
shared symlink (same fixed path for every sample), and Quarto
renders in-place next to the .qmd. Concurrent per-sample tasks
raced on that single physical directory ("cannot open file
per_sample.qmd"). Fixed with stageInMode 'copy' for an isolated
copy per task.
Snapshot regenerated (additive: LRSOMATICREPORT versions entry +
sample*/report/*_report.html); tests/.nftignore excludes the
Quarto-rendered HTML's unstable content, matching multiqc/nanoplot.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previously pinned tag (4506737a6b63b769) was built during a
singularity.enabled=true Wave session, which only produces a
Singularity-native SIF artifact -- Docker CI's docker|25.04.0 job
failed to pull it ("Encountered remote
application/vnd.sylabs.sif.config.v1+json (unknown) when fetching").
A second Wave freeze build under a docker-context session
(docker.enabled=true, wave.strategy=['conda']) produced tag
9d12b9297c3c4d38, a genuine OCI image (verified via
`skopeo inspect --raw`: application/vnd.oci.image.manifest.v1+json
with real tar+gzip layers). The dual-engine container directive now
uses this new tag for the docker branch; the singularity branch's
blob URL is unchanged (already confirmed working).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
stageInMode 'copy' (added to fix a race condition where concurrent per-sample Quarto renders collided on a shared symlinked report_src directory) has a real bug in Nextflow 25.04.0 for directory-type path inputs under the docker executor: docker|latest-everything passed but docker|25.04.0 failed with "cannot open file lrsomatic_report/bin/render_report.R: No such file or directory" -- the copied directory came out incomplete. Replaced the process-level directive with a plain `cp -rL` in the script body itself. This is pure shell with no Nextflow-version dependency, and fixes the same underlying problem: each task now dereferences report_src into its own private, task-local copy before Quarto renders in-place next to the .qmd. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment on lines
+561
to
+564
| withName : '.*:LRSOMATICREPORT' { | ||
| ext.prefix = { "${meta.id}" } | ||
| ext.args = { params.report_gene_panel ? "--gene-panel ${params.report_gene_panel}" : '' } | ||
| publishDir = [ |
Comment on lines
+10
to
+12
| container "${workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container | ||
| ? 'https://community-cr-prod.seqera.io/docker/registry/v2/blobs/sha256/e0/e0d4fabb2f79dcc0d3446f1bda84507eb52ac21ebea75fd29ee5b1b26c61ee34/data' | ||
| : 'community.wave.seqera.io/library/r-base_quarto_r-data.table_r-dplyr_pruned:9d12b9297c3c4d38'}" |
…Wakhan The `assets/lrsomatic_report` submodule could not reach anyone. `nextflow run IntGenomicsLab/lrsomatic` clones the pipeline repo but not its submodules, and CI checks out without `submodules: recursive` -- so the gitlink resolved to an empty directory for end users and for every CI run, which is what has been failing PR #176. Replace it with the upstream tree as real tracked files (bin/, R/, templates/, assets/, LICENSE, README.md; ~565 KB), recorded in assets/lrsomatic_report/VENDORED.md. `--report_src` stays, now as an override for a local checkout rather than a required setup step. Dependencies stay in the Wave multi-package container, rebuilt from the module's environment.yml after adding r-base64enc (used by R/utils.R embed_png(), listed in the upstream recipe, missing here). The tool is at v1.1.0, several releases past the pin. Rewire accordingly: - Drop the symlink tree that faked variants/clairs vs variants/clairsto so the old CLI could infer run mode. v1.1.0 derives the mode from whether normal-side QC is present and discovers files recursively by base name, so staging is now flat plus three fixed locations (qc/tumor, qc/normal, wakhan). - Drop `cp -rL` of report_src: render_report.R copies templates/ and assets/ into a task-local ._render itself, so the shared source dir is never written. - Feed the phased somatic VCF rather than the pre-phasing caller VCF, at the path the tool looks for it. The VAF/depth/phase-set columns now come from the same file VEP annotated instead of a possibly-consensus VCF. - Add SV_VEP.out.vcf, the tool's primary SV annotation source. - Add the Wakhan outputs it renders. Its per-solution plots all share one base name, so WAKHAN gains a `solution_dirs` output and the directories are staged whole rather than the files individually. - Export TMPDIR into the task work dir alongside HOME. Quarto's Deno runtime creates a session dir under TMPDIR and dies with "Read-only file system (os error 30): tmpdir" wherever the container's /tmp is not writable. The module test suite previously passed `checkIfExists` on a directory that existed but was empty, which is why it never caught any of this. It now has a stub test and a real-render test with a VEP somatic VCF, so a broken container, an incomplete tool tree or CLI drift all fail loudly. Refs #133 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The vendored tool tree needs a `linguist-vendored` entry so GitHub does not count 565 KB of upstream R and SCSS as pipeline source, but .gitattributes is template-managed and any edit fails files_unchanged. Opt it out the way the repo already opts out CODE_OF_CONDUCT.md and the workflow files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 57 changed files in this pull request and generated no new comments.
Suppressed comments (3)
modules/local/lrsomaticreport/main.nf:16
- The PR description says
container:points atcommunity.wave.seqera.io/library/r-base_quarto_r-data.table_r-dplyr_pruned:f1d36670d940c971, but this module hard-codes different Wave image tags/digests. Please align the PR description with the actual container references in code (or vice versa) so users know which image is expected to work.
workflows/lrsomatic.nf:961 ASCAT.out.pngcan emit a list of PNGs for a sample. After.mix(...).groupTuple()that can produce nested lists (e.g.[file, [png1,png2]]), which then gets passed downstream asascat_files. Flattening here (as you already do for Wakhan) keeps the contract consistently[meta, [file, ...]]and avoids surprising staging behavior inLRSOMATICREPORT.
workflows/lrsomatic.nf:545- The updated comment describes
meta.paired_dataas a boolean, but elsewhere it’s treated as a truthy/falsy value (it may be an id string for the paired sample). To avoid misleading future changes, reword the comment to describe the truthiness contract rather than a strict boolean.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 57 changed files in this pull request and generated no new comments.
Suppressed comments (3)
conf/modules.config:563
ext.argspasses--gene-panelwithout quoting the parameter value. If--report_gene_panelis a file path containing spaces (common on shared filesystems) or shell metacharacters, the rendered command line will break or be mis-parsed. Quote the value so it is passed as a single argument.
modules/local/lrsomaticreport/main.nf:16- The PR description states the report module uses a specific Wave image tag (
...r-data.table_r-dplyr_pruned:f1d36670d940c971), but the module is pinned to different tags here (...r-base64enc_r-data.table_pruned:dc62d809aa6fd497/...:c1049dbaf31bf178). This kind of drift can lead to hard-to-reproduce render failures if the pinned container doesn't matchenvironment.yml. Please align the PR documentation and the pinned image(s), and double-check the pinned image actually contains all R/Quarto deps listed inenvironment.yml.
assets/lrsomatic_report/VENDORED.md:10 - The PR description says
assets/lrsomatic_reportwas added as a git submodule pinned to a specific commit, but this file explicitly documents that the directory is vendored (not a submodule) and records different upstream/vendored SHAs. Please update the PR description to match the actual approach used in the diff so reviewers/users aren't misled about how the report source is managed.
This directory is a **vendored copy** of the standalone report tool, not a git submodule.
Do not edit it here — fix upstream, tag a release, and re-sync.
| | |
|---|---|
| Upstream | <https://github.com/ljwharbers/lrsomatic_report> |
| Release | `v1.1.0` (`9d660a77d5f23f92e1f7ff34f85da7956f445009`) |
| Vendored commit | `d17a636aeb3f79462b7f58db9102f4030941195b` (`main`) |
…ort-module # Conflicts: # CHANGELOG.md
Addresses the two review comments still live on PR #176. --report_gene_panel is documented as accepting a path to a TSV, but the raw param was interpolated straight into the command line: the file was never staged, so it was not bound into the docker/singularity container and render_report.R aborted with "--gene-panel not found". Add an optional `gene_panel` path input, wired from the workflow only when the param resolves to an existing file (a builtin panel name still travels via ext.args alone), and have conf/modules.config pass the quoted *base* name -- unchanged for a builtin, and the staged name for a TSV. Quoting also fixes panel paths containing spaces. The activation-hook loop hard-coded CONDA_PREFIX=/opt/conda, which is right for the Wave image but wrong under -profile conda, where it already points at the task's own env. Only fall back to /opt/conda when unset, and glob the hooks from $CONDA_PREFIX. Also pass checkIfExists to the report_src lookup so a bad --report_src fails fast, and cover the panel path with a real (non-stub) nf-test that renders with a user-supplied TSV -- it only passes if the file is genuinely staged into the container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment on lines
+150
to
+161
| all_p = if (!is.null(params$all_panels) && length(params$all_panels) > 0) | ||
| params$all_panels else list() | ||
| js_panels = paste0( | ||
| "const GENE_PANELS = {", | ||
| paste(vapply(names(all_p), function(nm) { | ||
| genes_json = paste0('"', all_p[[nm]], '"', collapse = ", ") | ||
| paste0('"', nm, '": new Set([', genes_json, '])') | ||
| }, character(1)), collapse = ",\n"), | ||
| "};\n", | ||
| 'const DEFAULT_PANEL = "', params$default_panel, '";\n' | ||
| ) | ||
| cat("<script>\n", js_panels, "\n</script>\n", sep = "") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wraps the lrsomatic_report R/Quarto tool as a new local module
LRSOMATICREPORTthat renders a self-contained per-sample HTML report, and runs it as the final pipeline step.assets/lrsomatic_reportis a vendored copy ofv1.1.0(9d660a77), taken atd17a636a; seeassets/lrsomatic_report/VENDORED.mdfor the rationale and the exact provenance.nextflow run IntGenomicsLab/lrsomaticclones the pipeline but does not initialise submodules, so a submodule would leave the default--report_srcpointing at an empty directory.--skip_report(defaultfalse), consistent withskip_ascat/skip_wakhan.remainder: true, so any skipped or missing upstream step degrades gracefully to a "not available" section instead of breaking the join.--skip_report,--report_src(default: the vendored path),--report_gene_panel.solution_dirsoutput to the WAKHAN module so its per-solution copy-number plots can be staged into the report.Containers
The module pins two Wave builds from its own
environment.yml(dependencies only — R, Quarto and the tool's R packages; the tool itself is vendored):oras://community.wave.seqera.io/library/r-base_quarto_r-base64enc_r-data.table_pruned:dc62d809aa6fd497community.wave.seqera.io/library/r-base_quarto_r-base64enc_r-data.table_pruned:c1049dbaf31bf178Two separate builds are needed:
wave --singularityproduces a Singularity-native SIF (theoras://reference), the default build a genuine OCI image. Rebuild both wheneverenvironment.ymlchanges.Notable fixes made along the way
environment.ymlwas originally derived from the tool's README package list; grepping the actual R source showedGenomicRanges/ComplexHeatmap/tidyrare unused, so they were dropped. This also sidesteps a real bioconda/micromamba incompatibility —GenomeInfoDbData's post-link data-fetch script never runs under micromamba.activate.dhooks (Quarto needsQUARTO_SHARE_PATH), so the module sources them explicitly.CONDA_PREFIXis exported first because one hook references it unguarded underset -u, but it now defaults to the existing value and only falls back to/opt/condawhen unset — hard-coding it broke-profile conda.--report_gene_panelaccepts a builtin panel name or a path to a TSV. The path is now declared as an optionalpath(gene_panel)module input so Nextflow stages it and it is bound into the container; previously it was interpolated into the command line as a bare string and the tool aborted with "--gene-panel not found". The argument is also quoted, so panel paths containing spaces work.HOMEandTMPDIRare pointed at the task work dir so Quarto/Deno's cache and session directories can't depend on the container's$HOMEor a read-only/tmp.Known limitations
somatic_vcffed to the report is the final (possibly consensus) somatic VCF, staged undervariants/phased/purely to drive the report's run-mode detection and VAF column; if multiple somatic callers were combined via consensus, that VAF column won't reflect a single real caller. The VEP-based variant table (the primary source) is unaffected.Test plan
nextflow config .validates;--report_gene_panelresolves to the expected--gene-panelargumentnextflow lintclean — no new warnings or errors introducedpre-commit run --all-filescleannf-test, stubnf-test, real render under-profile singularity— renders a valid HTML report from a VEP somatic VCFnf-test, real render with a user-supplied gene panel TSV — passes only if the panel file is genuinely staged into the container, which is the regression this PR fixesnf-teston CI (docker | 25.04.0anddocker | latest-everything)🤖 Generated with Claude Code